Skip to content

Fix Windows tool navigation and managed browser reuse - #27

Merged
matt-greathouse merged 3 commits into
masterfrom
fix-tool-navigation
Aug 8, 2026
Merged

Fix Windows tool navigation and managed browser reuse#27
matt-greathouse merged 3 commits into
masterfrom
fix-tool-navigation

Conversation

@matt-greathouse

Copy link
Copy Markdown
Contributor

Summary

  • Improve Windows application resolution and native text input handling
  • Update daemon and Lua prelude navigation behavior
  • Add managed browser reuse coverage and refresh Windows build/documentation support

Testing

  • Added or updated CLI, core, and managed-browser reuse tests
  • Not run (not requested)

@matt-greathouse matt-greathouse left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full diff (12 files, ~1000 added lines) across the Windows app resolver, desktop-session detection, native input hardening, managed-browser navigation Lua, and the Windows build/signing script. Ten findings inline, roughly in severity order.

The one to look at first is DaemonTextInput.cpp:160. The new ResolveKeycode pre-validation runs on all platforms, but the Linux implementation is a stub that returns -1 unconditionally — Linux input actually flows through NormalizeKey/XSendHotkey inside SendHotkey. Every press on Linux now fails with invalid_key. The existing tests are negative-only, so CI won't catch it.

Three other behavior regressions worth resolving before merge:

  • RDP sessions lose native control (PlatformWindows.cpp:546) — turning on real console-session detection makes onConsole false under RDP, failing window_activate, app_activate_pid, and managed-browser launch with desktop_session_unavailable.
  • focus_app injects a stray Shift on every call (LuaPrelude.cpp:2302) — force = true bypasses the daemon's alreadyReady short-circuit, so macOS posts a real key event even on an awake, unlocked desktop.
  • navigate tests for "URL changed" instead of "URL matches" (LuaPrelude.cpp:2184) — false failure when the target canonicalizes to the current URL, false success on any unrelated redirect during polling.

The rest are narrower: exe names dropped from GetFrontmostApp (breaks wait { frontmost = "notepad.exe" }, and so the confirm half of focus_app("notepad.exe")), a lost pid on the ShellExecute fast path, continue-on-activation-failure falling through to a duplicate launch, an app-catalog cache that never caches an empty result on a 200 ms poll path, a possibly-missing propsys link (unverified — worth checking the Windows CI job), and a post-sign Status -eq "Valid" check that a self-signed dev cert can't satisfy.

The direction of the PR is good — the activation retry, the input_failed vs invalid_key split, and the AUMID-based catalog matching are all real improvements. Most findings are about edges those changes opened up rather than the approach itself.


Generated by Claude Code

Comment thread src/daemon/DaemonTextInput.cpp
Comment thread src/platform/windows/PlatformWindows.cpp Outdated
Comment thread src/cli/LuaPrelude.cpp Outdated
Comment thread src/cli/LuaPrelude.cpp
Comment thread src/platform/windows/PlatformWindows.cpp
Comment thread src/platform/windows/PlatformWindows.cpp
Comment thread src/platform/windows/PlatformWindows.cpp
Comment thread src/platform/windows/PlatformWindows.cpp Outdated
Comment thread CMakeLists.txt
Comment thread scripts/build-windows.ps1 Outdated
@matt-greathouse
matt-greathouse merged commit 0ba3465 into master Aug 8, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant